-
Notifications
You must be signed in to change notification settings - Fork 421
Add an experimental +1000 offset to the 0FC feature bit #4108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add an experimental +1000 offset to the 0FC feature bit #4108
Conversation
👋 Thanks for assigning @TheBlueMatt as a reviewer! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4108 +/- ##
==========================================
- Coverage 88.72% 88.69% -0.03%
==========================================
Files 177 177
Lines 133286 133294 +8
Branches 133286 133294 +8
==========================================
- Hits 118253 118225 -28
- Misses 12326 12360 +34
- Partials 2707 2709 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
lightning-types/src/features.rs
Outdated
); | ||
define_feature!( | ||
1041, // The BOLTs PR uses feature bit 40/41, so add +1000 for the experimental bit | ||
AnchorZeroFeeCommitments, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we rename this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done below
👋 The first review has been submitted! Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer. |
9f20993
to
314c98d
Compare
lightning-types/src/features.rs
Outdated
); | ||
define_feature!( | ||
1041, // The BOLTs PR uses feature bit 40/41, so add +1000 for the experimental bit | ||
ZeroFeeCommitments, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Err, sorry, but "different name" I didn't really mean "drop the Anchor" part, but rather "something like AnchorZeroFeeCommitmentsPreSpec or AnchorZeroFeeCommitmentsLDKTesting or AnchorZeroFeeCommitmentsTestingPhase or..."
Silly question - are we sure we want to add 1000 for experimental features? I could have sworn that we agreed to add 100 in a spec meeting to stay within the "protocol reserved" range (>255 are usable by BLIPs) but now can't find that written down anywhere. |
No idea lol. +100 seems fine to me too 🤷♂️ |
Thank you @carlaKC yes I went with +1000 to match the |
Want to move the HoldHtlc bit to +100 while you're at it? |
314c98d
to
6008e04
Compare
Moving the Hold bit one sec |
We prefer to stay within the 0-255 range reserved for bolts, as bits > 255 can be used by bLIPs.
A little heads up @valentinewallace we are moving the staging |
Landing as trivial. |
Address #4062